Open
Conversation
WalkthroughThis update introduces a batch donation feature to the campaign donation contract. It adds new data structures and events for tracking batch donation outcomes, implements a gas-optimized batch donation method, and updates the contract interface to support batch processing and event emission for multiple campaign donations in a single transaction. Changes
Sequence Diagram(s)sequenceDiagram
participant Donor
participant Contract
participant Campaigns
Donor->>Contract: batch_donate([(campaign_id, amount), ...])
Contract->>Contract: Validate input and aggregate total
Contract->>Donor: Assert balance and allowance
Donor->>Contract: Transfer total amount
loop For each (campaign_id, amount)
Contract->>Campaigns: Process donation (auto-cap if needed)
Campaigns-->>Contract: Return donation result
end
Contract->>Donor: Refund excess (if any)
Contract-->>Donor: Emit BatchDonationProcessed event
Possibly related issues
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/base/errors.cairo (1)
142-143: Minor comment consistency improvement needed.The error constant is well-defined, but the comment should follow the documentation pattern used elsewhere in the file.
- // Throw Error when Total amount must be > 0 + /// Thrown when the total donation amount in a batch is zero or negative pub const TOTAL_DONATION_AMOUNT: felt252 = 'Error: Total amount must be > 0';src/campaign_donation.cairo (1)
460-537: Well-implemented batch donation functionality.The batch donation implementation is well-designed with:
- Appropriate batch size limit
- Gas-optimized single token transfer
- Proper auto-capping and refund handling
- Comprehensive event emission
Minor suggestion: Consider making
MAX_BATCH_SIZEa storage variable for future flexibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/base/errors.cairo(1 hunks)src/base/types.cairo(1 hunks)src/campaign_donation.cairo(5 hunks)src/interfaces/ICampaignDonation.cairo(1 hunks)
🔇 Additional comments (4)
src/base/types.cairo (2)
88-94: Well-designed struct for batch donation results.The
DonationResultstruct is properly designed with appropriate traits (Drop,Serde,Clone) and fields to track individual donation outcomes within a batch operation.
96-104: Comprehensive event structure for batch donations.The
BatchDonationProcessedevent is well-structured with the donor field appropriately keyed for efficient filtering, and includes all necessary fields to track batch donation outcomes.src/campaign_donation.cairo (2)
27-31: Import additions are appropriate.The new imports for
TOTAL_DONATION_AMOUNTerror andDonationResulttype are correctly added to support the batch donation feature.
995-1001: Standard upgradeable implementation.The upgrade function is correctly implemented with proper ownership validation.
Comment on lines
+227
to
+230
| fn batch_donate( | ||
| ref self: TContractState, | ||
| campaign_amounts: Array<(u256, u256)> // Array of (campaign_id, amount) | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add documentation for the batch_donate method.
The method signature is well-designed, but it lacks the comprehensive documentation that other methods in this interface have. Please add documentation to maintain consistency.
+ /// Makes donations to multiple campaigns in a single transaction
+ ///
+ /// # Arguments
+ /// * `campaign_amounts` - Array of tuples containing (campaign_id, amount) pairs
+ ///
+ /// # Requirements
+ /// * Array must not be empty
+ /// * Array size must not exceed maximum batch size
+ /// * Total amount must be greater than zero
+ /// * Donor must have approved the contract for the total amount
+ /// * Each campaign must exist and be active
+ ///
+ /// # Effects
+ /// * Transfers total amount from donor to contract in single transaction
+ /// * Processes each donation individually with auto-capping
+ /// * Refunds any excess if campaigns are capped
+ /// * Emits BatchDonationProcessed event with detailed results
fn batch_donate(
ref self: TContractState,
campaign_amounts: Array<(u256, u256)> // Array of (campaign_id, amount)
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fn batch_donate( | |
| ref self: TContractState, | |
| campaign_amounts: Array<(u256, u256)> // Array of (campaign_id, amount) | |
| ); | |
| /// Makes donations to multiple campaigns in a single transaction | |
| /// | |
| /// # Arguments | |
| /// * `campaign_amounts` - Array of tuples containing (campaign_id, amount) pairs | |
| /// | |
| /// # Requirements | |
| /// * Array must not be empty | |
| /// * Array size must not exceed maximum batch size | |
| /// * Total amount must be greater than zero | |
| /// * Donor must have approved the contract for the total amount | |
| /// * Each campaign must exist and be active | |
| /// | |
| /// # Effects | |
| /// * Transfers total amount from donor to contract in single transaction | |
| /// * Processes each donation individually with auto-capping | |
| /// * Refunds any excess if campaigns are capped | |
| /// * Emits BatchDonationProcessed event with detailed results | |
| fn batch_donate( | |
| ref self: TContractState, | |
| campaign_amounts: Array<(u256, u256)> // Array of (campaign_id, amount) | |
| ); |
🤖 Prompt for AI Agents
In src/interfaces/ICampaignDonation.cairo around lines 227 to 230, the
batch_donate method lacks documentation unlike other methods in this interface.
Add a clear docstring above the method signature describing its purpose,
parameters (campaign_amounts as an array of (campaign_id, amount) tuples), and
any relevant behavior or return information to maintain consistency with the
rest of the interface.
Comment on lines
+874
to
992
| fn _validate_and_calculate_total( | ||
| self: @ContractState, campaign_amounts: @Array<(u256, u256)>, | ||
| ) -> u256 { | ||
| let mut total: u256 = 0; | ||
| let mut i = 0; | ||
|
|
||
| while i < campaign_amounts.len() { | ||
| let (campaign_id, amount) = *campaign_amounts.at(i); | ||
|
|
||
| // Validate donation amount > 0 | ||
| assert(amount > 0, 'Amount must be > 0'); | ||
|
|
||
| // Validate campaign exists | ||
| let campaign = self.campaigns.read(campaign_id); | ||
| assert(!campaign.owner.is_zero(), 'Campaign does not exist'); | ||
|
|
||
| // Check campaign is active (not closed/goal reached) | ||
| assert(!campaign.is_closed, 'Campaign is closed'); | ||
| assert(!campaign.is_goal_reached, 'Campaign goal reached'); | ||
|
|
||
| // Check for integer overflow in total calculation | ||
| let new_total = total + amount; | ||
| assert(new_total >= total, 'Amount overflow'); | ||
| total = new_total; | ||
|
|
||
| i += 1; | ||
| } | ||
|
|
||
| total | ||
| } | ||
|
|
||
| /// GAS-OPTIMIZED: O(n) validation that handles mid-batch campaign completion | ||
| /// Pre-calculates campaign totals to avoid nested loops (was O(n²), now O(n)) | ||
| /// FIXED: Removed dictionary approach to avoid Copy trait issues | ||
| fn _validate_and_calculate_total_optimized( | ||
| self: @ContractState, campaign_amounts: @Array<(u256, u256)>, | ||
| ) -> u256 { | ||
| // STEP 1: Pre-calculate campaign batch totals in single pass O(n) | ||
| let mut campaign_totals: Array<CampaignBatchTotal> = ArrayTrait::new(); | ||
| let mut i = 0; | ||
|
|
||
| while i < campaign_amounts.len() { | ||
| let (campaign_id, amount) = *campaign_amounts.at(i); | ||
|
|
||
| // Validate donation amount > 0 | ||
| assert(amount > 0, 'Amount must be > 0'); | ||
|
|
||
| // Find existing total for this campaign or create new entry | ||
| let mut found = false; | ||
| let mut found_index = 0; | ||
| let mut j = 0; | ||
| while j < campaign_totals.len() { | ||
| let existing_total = *campaign_totals.at(j); // FIXED: Now works with Copy trait | ||
| if existing_total.campaign_id == campaign_id { | ||
| found = true; | ||
| found_index = j; | ||
| break; | ||
| } | ||
| j += 1; | ||
| } | ||
| if token_name == 'USDT' || token_name == 'usdt' { | ||
| token_address = | ||
| contract_address_const::< | ||
| 0x068f5c6a61780768455de69077e07e89787839bf8166decfbf92b645209c0fb8, | ||
| >(); | ||
|
|
||
| if found { | ||
| // Update existing total (simplified approach - rebuild array) | ||
| let mut new_totals: Array<CampaignBatchTotal> = ArrayTrait::new(); | ||
| let mut k = 0; | ||
| while k < campaign_totals.len() { | ||
| let existing = *campaign_totals.at(k); | ||
| if k == found_index { | ||
| new_totals | ||
| .append( | ||
| CampaignBatchTotal { | ||
| campaign_id: existing.campaign_id, | ||
| total_amount: existing.total_amount + amount, | ||
| }, | ||
| ); | ||
| } else { | ||
| new_totals.append(existing); | ||
| } | ||
| k += 1; | ||
| } | ||
| campaign_totals = new_totals; | ||
| } else { | ||
| campaign_totals.append(CampaignBatchTotal { campaign_id, total_amount: amount }); | ||
| } | ||
|
|
||
| token_address | ||
| i += 1; | ||
| } | ||
| } | ||
|
|
||
| #[abi(embed_v0)] | ||
| impl UpgradeableImpl of IUpgradeable<ContractState> { | ||
| fn upgrade(ref self: ContractState, new_class_hash: ClassHash) { | ||
| self.ownable.assert_only_owner(); | ||
| self.upgradeable.upgrade(new_class_hash); | ||
| // STEP 2: Validate campaigns and calculate effective total O(unique_campaigns) | ||
| let mut total: u256 = 0; | ||
| let mut k = 0; | ||
|
|
||
| while k < campaign_totals.len() { | ||
| let campaign_total = *campaign_totals.at(k); // FIXED: Now works with Copy trait | ||
|
|
||
| // Validate campaign exists and is active | ||
| let campaign = self.campaigns.read(campaign_total.campaign_id); | ||
| assert(!campaign.owner.is_zero(), 'Campaign does not exist'); | ||
| assert(!campaign.is_closed, 'Campaign is closed'); | ||
| assert(!campaign.is_goal_reached, 'Campaign goal reached'); | ||
|
|
||
| // Calculate effective amount with auto-capping | ||
| let remaining = campaign.target_amount - campaign.current_balance; | ||
| let effective_amount = if campaign_total.total_amount > remaining { | ||
| remaining // Cap to remaining amount | ||
| } else { | ||
| campaign_total.total_amount // Use full amount | ||
| }; | ||
|
|
||
| total += effective_amount; | ||
|
|
||
| // Check for overflow | ||
| assert(total >= effective_amount, 'Total overflow'); | ||
|
|
||
| k += 1; | ||
| } | ||
|
|
||
| total | ||
| } |
There was a problem hiding this comment.
Remove duplicate/dead code.
This section contains duplicate implementations of validation functions that are not used. The _validate_and_calculate_total_optimized function is already defined at lines 562-647, and _validate_and_calculate_total appears to be an unused older version.
Remove this entire duplicate code section (lines 874-992) to improve code clarity and maintainability.
🤖 Prompt for AI Agents
In src/campaign_donation.cairo from lines 874 to 992, there is a duplicate
implementation of the validation functions where _validate_and_calculate_total
and _validate_and_calculate_total_optimized are redefined. Since
_validate_and_calculate_total_optimized is already defined earlier at lines
562-647 and the older _validate_and_calculate_total is unused, remove the entire
code block from lines 874 to 992 to eliminate redundancy and improve code
clarity.
Comment on lines
+562
to
+647
| fn _validate_and_calculate_total_optimized( | ||
| self: @ContractState, campaign_amounts: @Array<(u256, u256)>, | ||
| ) -> u256 { | ||
| // STEP 1: Pre-calculate campaign batch totals in single pass O(n) | ||
| let mut campaign_totals: Array<CampaignBatchTotal> = ArrayTrait::new(); | ||
| let mut i = 0; | ||
|
|
||
| while i < campaign_amounts.len() { | ||
| let (campaign_id, amount) = *campaign_amounts.at(i); | ||
|
|
||
| // Validate donation amount > 0 | ||
| assert(amount > 0, 'Amount must be > 0'); | ||
|
|
||
| // Find existing total for this campaign or create new entry | ||
| let mut found = false; | ||
| let mut found_index = 0; | ||
| let mut j = 0; | ||
| while j < campaign_totals.len() { | ||
| let existing_total = *campaign_totals.at(j); // FIXED: Now works with Copy trait | ||
| if existing_total.campaign_id == campaign_id { | ||
| found = true; | ||
| found_index = j; | ||
| break; | ||
| } | ||
| j += 1; | ||
| } | ||
|
|
||
| if found { | ||
| // Update existing total (simplified approach - rebuild array) | ||
| let mut new_totals: Array<CampaignBatchTotal> = ArrayTrait::new(); | ||
| let mut k = 0; | ||
| while k < campaign_totals.len() { | ||
| let existing = *campaign_totals.at(k); | ||
| if k == found_index { | ||
| new_totals | ||
| .append( | ||
| CampaignBatchTotal { | ||
| campaign_id: existing.campaign_id, | ||
| total_amount: existing.total_amount + amount, | ||
| }, | ||
| ); | ||
| } else { | ||
| new_totals.append(existing); | ||
| } | ||
| k += 1; | ||
| } | ||
| campaign_totals = new_totals; | ||
| } else { | ||
| campaign_totals | ||
| .append(CampaignBatchTotal { campaign_id, total_amount: amount }); | ||
| } | ||
|
|
||
| i += 1; | ||
| } | ||
|
|
||
| // STEP 2: Validate campaigns and calculate effective total O(unique_campaigns) | ||
| let mut total: u256 = 0; | ||
| let mut k = 0; | ||
|
|
||
| while k < campaign_totals.len() { | ||
| let campaign_total = *campaign_totals.at(k); // FIXED: Now works with Copy trait | ||
|
|
||
| // Validate campaign exists and is active | ||
| let campaign = self.campaigns.read(campaign_total.campaign_id); | ||
| assert(!campaign.owner.is_zero(), 'Campaign does not exist'); | ||
| assert(!campaign.is_closed, 'Campaign is closed'); | ||
| assert(!campaign.is_goal_reached, 'Campaign goal reached'); | ||
|
|
||
| // Calculate effective amount with auto-capping | ||
| let remaining = campaign.target_amount - campaign.current_balance; | ||
| let effective_amount = if campaign_total.total_amount > remaining { | ||
| remaining // Cap to remaining amount | ||
| } else { | ||
| campaign_total.total_amount // Use full amount | ||
| }; | ||
|
|
||
| total += effective_amount; | ||
|
|
||
| // Check for overflow | ||
| assert(total >= effective_amount, 'Total overflow'); | ||
|
|
||
| k += 1; | ||
| } | ||
|
|
||
| total | ||
| } |
There was a problem hiding this comment.
Missing struct definition for CampaignBatchTotal.
The function uses a CampaignBatchTotal struct that is not defined in any of the provided files. This will cause a compilation error.
Add the missing struct definition:
#[derive(Copy, Drop)]
struct CampaignBatchTotal {
campaign_id: u256,
total_amount: u256,
}🤖 Prompt for AI Agents
In src/campaign_donation.cairo around lines 562 to 647, the struct
CampaignBatchTotal used in the function _validate_and_calculate_total_optimized
is missing its definition, causing a compilation error. Define the struct
CampaignBatchTotal with fields campaign_id and total_amount both of type u256,
and derive Copy and Drop traits as follows: #[derive(Copy, Drop)] struct
CampaignBatchTotal { campaign_id: u256, total_amount: u256 }. Add this
definition before the function or in an appropriate shared location.
Comment on lines
+87
to
+95
| #[derive(Drop, starknet::Event)] | ||
| pub struct BatchDonationProcessed { | ||
| #[key] | ||
| pub donor: ContractAddress, | ||
| pub total_campaigns: u32, | ||
| pub successful_donations: u32, | ||
| pub total_amount: u256, | ||
| pub results: Array<DonationResult>, | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove duplicate event definition.
The BatchDonationProcessed event is already defined in src/base/types.cairo. Consider importing it instead of duplicating the definition.
- #[derive(Drop, starknet::Event)]
- pub struct BatchDonationProcessed {
- #[key]
- pub donor: ContractAddress,
- pub total_campaigns: u32,
- pub successful_donations: u32,
- pub total_amount: u256,
- pub results: Array<DonationResult>,
- }
+ use crate::base::types::BatchDonationProcessed;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #[derive(Drop, starknet::Event)] | |
| pub struct BatchDonationProcessed { | |
| #[key] | |
| pub donor: ContractAddress, | |
| pub total_campaigns: u32, | |
| pub successful_donations: u32, | |
| pub total_amount: u256, | |
| pub results: Array<DonationResult>, | |
| } | |
| use crate::base::types::BatchDonationProcessed; |
🤖 Prompt for AI Agents
In src/campaign_donation.cairo around lines 87 to 95, the BatchDonationProcessed
event struct is duplicated but already defined in src/base/types.cairo. Remove
this duplicate definition from campaign_donation.cairo and instead import
BatchDonationProcessed from src/base/types.cairo to avoid redundancy and
maintain consistency.
Comment on lines
+778
to
+833
| fn _process_internal_donation_with_return( | ||
| ref self: ContractState, donor: ContractAddress, campaign_id: u256, amount: u256, | ||
| ) -> (u256, u256) { | ||
| let mut campaign = self.campaigns.read(campaign_id); | ||
| let timestamp = get_block_timestamp(); | ||
|
|
||
| // Calculate actual donation amount (don't exceed target) - AUTO-CAPPING | ||
| let remaining_amount = campaign.target_amount - campaign.current_balance; | ||
| let actual_amount = if amount > remaining_amount { | ||
| remaining_amount | ||
| } else { | ||
| amount | ||
| }; | ||
|
|
||
| fn get_asset_address(self: @ContractState, token_name: felt252) -> ContractAddress { | ||
| let mut token_address: ContractAddress = contract_address_const::<0>(); | ||
| if token_name == 'USDC' || token_name == 'usdc' { | ||
| token_address = | ||
| contract_address_const::< | ||
| 0x053c91253bc9682c04929ca02ed00b3e423f6710d2ee7e0d5ebb06f3ecf368a8, | ||
| >(); | ||
| // Skip if no amount to donate (campaign already fully funded) | ||
| if actual_amount == 0 { | ||
| return (0, 0); | ||
| } | ||
| if token_name == 'STRK' || token_name == 'strk' { | ||
| token_address = | ||
| contract_address_const::< | ||
| 0x04718f5a0fc34cc1af16a1cdee98ffb20c31f5cd61d6ab07201858f4287c938d, | ||
| >(); | ||
|
|
||
| // Get next donation ID | ||
| let donation_id = self.donation_count.read() + 1; | ||
|
|
||
| // Update campaign amount | ||
| campaign.current_balance = campaign.current_balance + actual_amount; | ||
|
|
||
| // If goal reached, mark as closed | ||
| if campaign.current_balance >= campaign.target_amount { | ||
| campaign.is_goal_reached = true; | ||
| campaign.is_closed = true; | ||
| } | ||
| if token_name == 'ETH' || token_name == 'eth' { | ||
| token_address = | ||
| contract_address_const::< | ||
| 0x049d36570d4e46f48e99674bd3fcc84644ddd6b96f7c741b1562b82f9e004dc7, | ||
| >(); | ||
|
|
||
| self.campaigns.write(campaign_id, campaign); | ||
|
|
||
| // Create donation record | ||
| let donation = Donations { donation_id, donor, campaign_id, amount: actual_amount }; | ||
|
|
||
| // Properly append to the Vec using push | ||
| self.donations.entry(campaign_id).push(donation); | ||
|
|
||
| self.donation_count.write(donation_id); | ||
|
|
||
| // Update the per-campaign donation count | ||
| let campaign_donation_count = self.donation_counts.read(campaign_id); | ||
| self.donation_counts.write(campaign_id, campaign_donation_count + 1); | ||
|
|
||
| // Emit donation event for each successful donation | ||
| self | ||
| .emit( | ||
| Event::Donation( | ||
| Donation { donor, campaign_id, amount: actual_amount, timestamp }, | ||
| ), | ||
| ); | ||
|
|
||
| // Return both donation_id and actual_amount for tracking | ||
| (donation_id, actual_amount) | ||
| } |
There was a problem hiding this comment.
Missing donor donation tracking update.
The function processes donations but doesn't update the donor_donations mapping that tracks donations by donor, unlike the original _donate_to_campaign function.
Add the missing donor tracking after line 821:
self.donation_counts.write(campaign_id, campaign_donation_count + 1);
+
+ // Save donation reference for the donor
+ self.donor_donations.entry(donor).push((campaign_id, donation_id));
+
+ // Update donor's total donation for this campaign
+ let current_total = self.donations_by_donor.read((campaign_id, donor));
+ self.donations_by_donor.write((campaign_id, donor), current_total + actual_amount);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/campaign_donation.cairo between lines 778 and 833, the function
_process_internal_donation_with_return correctly processes donations but does
not update the donor_donations mapping to track donations by donor. To fix this,
after line 821 where the donation is appended to
self.donations.entry(campaign_id), add code to append the donation to
self.donor_donations.entry(donor) to maintain accurate donor donation tracking.
Utilitycoder
requested changes
Jul 31, 2025
Contributor
Utilitycoder
left a comment
Utilitycoder
left a comment
There was a problem hiding this comment.
Please review the code changes following some of the suggestions made by coderabbit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes issue: #102
Summary by CodeRabbit